-
-
Notifications
You must be signed in to change notification settings - Fork 445
Add Theme mode API Function #3797
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds support for a dark/light theme API so external plugins can query and react to Flow Launcher’s theme changes.
- Wire up ModernWpf theme change events in the main view model and expose them.
- Surface
IsApplicationDarkTheme()
andActualApplicationThemeChanged
in the public API. - Update
MainWindow
to listen to the view model’s theme event instead of subscribing directly.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
ViewModel/MainViewModel.cs | Subscribes/unsubscribes to ThemeManager.Current and raises a plugin event with theme info. |
PublicAPIInstance.cs | Implements IsApplicationDarkTheme() and forwards the theme-changed event to plugins. |
MainWindow.xaml.cs | Replaces direct ThemeManager subscription with the VM’s event. |
Plugin/Interfaces/IPublicAPI.cs | Adds IsApplicationDarkTheme() and ActualApplicationThemeChanged to the public interface. |
Plugin/EventHandler.cs | Defines ActualApplicationThemeChangedEventHandler and its EventArgs . |
Comments suppressed due to low confidence (4)
Flow.Launcher/ViewModel/MainViewModel.cs:206
- Passing
Application.Current
as the event sender may be misleading to subscribers. Consider usingthis
(the view model) or the originalsender
to clearly identify the source of the event.
Application.Current,
Flow.Launcher/PublicAPIInstance.cs:591
- There are no existing unit tests for this new API method. Consider adding tests to verify both light and dark theme states and ensure this stays correct as theme logic evolves.
public bool IsApplicationDarkTheme()
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs:606
- [nitpick] The remark about 'plugin will continue to be subscribed' is an implementation detail. Consider moving that note to a
<remarks>
section or the README, so the summary remains focused on the event’s purpose.
/// Invoked when the actual theme of the application has changed. Currently, the plugin will continue to be subscribed even if it is turned off.
Flow.Launcher/PublicAPIInstance.cs:596
- Exposed events can lead to memory leaks if subscribers don’t unsubscribe. Consider using weak event patterns or automatically removing handlers when plugins unload.
public event ActualApplicationThemeChangedEventHandler ActualApplicationThemeChanged
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. Forbidden patterns 🙅 (1)In order to address this, you could change the content to not match the forbidden patterns (comments before forbidden patterns may help explain why they're forbidden), add patterns for acceptable instances, or adjust the forbidden patterns themselves. These forbidden patterns matched content: s.b. workaround(s)
If the flagged items are 🤯 false positivesIf items relate to a ...
|
🥷 Code experts: onesounds Jack251970, onesounds have most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughThe changes introduce a new event and related event arguments for application theme changes, integrate ModernWpf theme management into the ViewModel, and expose theme status and change events via the public API. Event handler subscriptions in MainWindow are updated to use the ViewModel event instead of the static ThemeManager event. Changes
Sequence Diagram(s)sequenceDiagram
participant ThemeManager
participant MainViewModel
participant MainWindow
participant PublicAPI
ThemeManager->>MainViewModel: ActualApplicationThemeChanged
MainViewModel->>MainWindow: ActualApplicationThemeChanged event
MainWindow->>MainWindow: ViewModel_ActualApplicationThemeChanged handler
MainViewModel->>PublicAPI: ActualApplicationThemeChanged event (public)
PublicAPI->>Plugins: ActualApplicationThemeChanged event (exposed via IPublicAPI)
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Flow.Launcher.Plugin/EventHandler.cs (1)
88-97
: Fix inaccurate documentation comment.The documentation for the
IsDark
property is misleading. It should describe what the property represents, not when it's true./// <summary> - /// <see langword="true"/> if the application has changed actual theme + /// <see langword="true"/> if the application is using a dark theme /// </summary> public bool IsDark { get; init; }Flow.Launcher/ViewModel/MainViewModel.cs (1)
203-211
: Consider usingthis
as the event sender for better clarity.The event handler implementation is correct, but passing
Application.Current
as the sender might be confusing since the public event is owned by the MainViewModel. Consider usingthis
instead for consistency.private void ThemeManager_ActualApplicationThemeChanged(ThemeManager sender, object args) { ActualApplicationThemeChanged?.Invoke( - Application.Current, + this, new ActualApplicationThemeChangedEventArgs() { IsDark = sender.ActualApplicationTheme == ApplicationTheme.Dark }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Flow.Launcher.Plugin/EventHandler.cs
(2 hunks)Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs
(1 hunks)Flow.Launcher/MainWindow.xaml.cs
(4 hunks)Flow.Launcher/PublicAPIInstance.cs
(2 hunks)Flow.Launcher/ViewModel/MainViewModel.cs
(4 hunks)
🧰 Additional context used
🧠 Learnings (5)
Flow.Launcher/MainWindow.xaml.cs (9)
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3672
File: Flow.Launcher/MainWindow.xaml.cs:318-318
Timestamp: 2025-06-08T14:12:12.842Z
Learning: In Flow.Launcher, the App.NotifyIcon static property is initialized in the App class before MainWindow creation, so null checks are not needed when accessing App.NotifyIcon in MainWindow lifecycle methods.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Learnt from: onesounds
PR: Flow-Launcher/Flow.Launcher#0
File: :0-0
Timestamp: 2025-04-23T15:14:49.986Z
Learning: In WPF applications like Flow.Launcher, font styling should be applied using implicit styles instead of setting the FontFamily property on individual controls. Define implicit styles in a ResourceDictionary using <Style TargetType="{x:Type Button}"> format and merge it into App.xaml, which automatically applies the font to all instances of the control type while still allowing explicit overrides where needed.
Learnt from: Yusyuriv
PR: Flow-Launcher/Flow.Launcher#3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:40:11.014Z
Learning: In Flow Launcher, when using Windows Forms dialogs (e.g., in `JsonRPCPluginSettings.cs`), path validation is enabled by default in `OpenFileDialog` and `FolderBrowserDialog`, preventing users from selecting invalid paths, but it's possible to opt out of this validation on individual dialogs.
Learnt from: onesounds
PR: Flow-Launcher/Flow.Launcher#3394
File: Flow.Launcher/Themes/Darker Glass.xaml:134-141
Timestamp: 2025-03-28T21:12:13.386Z
Learning: In Flow.Launcher, hotkey styling is implemented with a two-component structure: a Border element with style `ItemHotkeyBGStyle` that provides background and border styling, containing a TextBlock with style `ItemHotkeyStyle` that handles the text styling.
Learnt from: jjw24
PR: Flow-Launcher/Flow.Launcher#2448
File: Plugins/Flow.Launcher.Plugin.Sys/ThemeSelector.cs:16-20
Timestamp: 2025-01-18T10:10:18.414Z
Learning: In Flow Launcher's plugin system, the PluginInitContext parameter passed to plugin constructors is guaranteed to be non-null by the plugin initialization system, making null checks unnecessary.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3279
File: Flow.Launcher/Helper/WallpaperPathRetrieval.cs:44-46
Timestamp: 2025-02-28T07:47:24.148Z
Learning: In Flow.Launcher's WallpaperPathRetrieval class, using a `using` statement with MemoryStream when loading images with BitmapImage does not work properly, even when using BitmapCacheOption.OnLoad. The stream needs to remain open while the bitmap is in use.
Learnt from: taooceros
PR: Flow-Launcher/Flow.Launcher#2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (1)
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Flow.Launcher.Plugin/EventHandler.cs (1)
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Flow.Launcher/ViewModel/MainViewModel.cs (10)
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3672
File: Flow.Launcher/MainWindow.xaml.cs:318-318
Timestamp: 2025-06-08T14:12:12.842Z
Learning: In Flow.Launcher, the App.NotifyIcon static property is initialized in the App class before MainWindow creation, so null checks are not needed when accessing App.NotifyIcon in MainWindow lifecycle methods.
Learnt from: Yusyuriv
PR: Flow-Launcher/Flow.Launcher#3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:40:11.014Z
Learning: In Flow Launcher, when using Windows Forms dialogs (e.g., in `JsonRPCPluginSettings.cs`), path validation is enabled by default in `OpenFileDialog` and `FolderBrowserDialog`, preventing users from selecting invalid paths, but it's possible to opt out of this validation on individual dialogs.
Learnt from: onesounds
PR: Flow-Launcher/Flow.Launcher#3394
File: Flow.Launcher/Themes/Darker Glass.xaml:134-141
Timestamp: 2025-03-28T21:20:54.978Z
Learning: In WPF applications like Flow.Launcher, Border elements cannot directly display text content and require a child element like TextBlock to handle text rendering. This separation of concerns (Border for visual container styling, TextBlock for text display) follows WPF best practices and provides greater styling flexibility.
Learnt from: onesounds
PR: Flow-Launcher/Flow.Launcher#3394
File: Flow.Launcher/Themes/Darker Glass.xaml:134-141
Timestamp: 2025-03-28T21:20:54.978Z
Learning: In WPF applications like Flow.Launcher, Border elements cannot directly display text content and require a child element like TextBlock to handle text rendering. This separation of concerns (Border for visual container styling, TextBlock for text display) follows WPF best practices and provides greater styling flexibility.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Learnt from: onesounds
PR: Flow-Launcher/Flow.Launcher#3394
File: Flow.Launcher/Themes/Darker Glass.xaml:134-141
Timestamp: 2025-03-28T21:12:13.386Z
Learning: In Flow.Launcher, hotkey styling is implemented with a two-component structure: a Border element with style `ItemHotkeyBGStyle` that provides background and border styling, containing a TextBlock with style `ItemHotkeyStyle` that handles the text styling.
Learnt from: onesounds
PR: Flow-Launcher/Flow.Launcher#0
File: :0-0
Timestamp: 2025-04-23T15:14:49.986Z
Learning: In WPF applications like Flow.Launcher, font styling should be applied using implicit styles instead of setting the FontFamily property on individual controls. Define implicit styles in a ResourceDictionary using <Style TargetType="{x:Type Button}"> format and merge it into App.xaml, which automatically applies the font to all instances of the control type while still allowing explicit overrides where needed.
Learnt from: taooceros
PR: Flow-Launcher/Flow.Launcher#2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Learnt from: jjw24
PR: Flow-Launcher/Flow.Launcher#2448
File: Plugins/Flow.Launcher.Plugin.Sys/ThemeSelector.cs:16-20
Timestamp: 2025-01-18T10:10:18.414Z
Learning: In Flow Launcher's plugin system, the PluginInitContext parameter passed to plugin constructors is guaranteed to be non-null by the plugin initialization system, making null checks unnecessary.
Flow.Launcher/PublicAPIInstance.cs (10)
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3672
File: Flow.Launcher/MainWindow.xaml.cs:318-318
Timestamp: 2025-06-08T14:12:12.842Z
Learning: In Flow.Launcher, the App.NotifyIcon static property is initialized in the App class before MainWindow creation, so null checks are not needed when accessing App.NotifyIcon in MainWindow lifecycle methods.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.
Learnt from: Yusyuriv
PR: Flow-Launcher/Flow.Launcher#3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:40:11.014Z
Learning: In Flow Launcher, when using Windows Forms dialogs (e.g., in `JsonRPCPluginSettings.cs`), path validation is enabled by default in `OpenFileDialog` and `FolderBrowserDialog`, preventing users from selecting invalid paths, but it's possible to opt out of this validation on individual dialogs.
Learnt from: onesounds
PR: Flow-Launcher/Flow.Launcher#0
File: :0-0
Timestamp: 2025-04-23T15:14:49.986Z
Learning: In WPF applications like Flow.Launcher, font styling should be applied using implicit styles instead of setting the FontFamily property on individual controls. Define implicit styles in a ResourceDictionary using <Style TargetType="{x:Type Button}"> format and merge it into App.xaml, which automatically applies the font to all instances of the control type while still allowing explicit overrides where needed.
Learnt from: jjw24
PR: Flow-Launcher/Flow.Launcher#2448
File: Plugins/Flow.Launcher.Plugin.Sys/ThemeSelector.cs:16-20
Timestamp: 2025-01-18T10:10:18.414Z
Learning: In Flow Launcher's plugin system, the PluginInitContext parameter passed to plugin constructors is guaranteed to be non-null by the plugin initialization system, making null checks unnecessary.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3279
File: Flow.Launcher/Helper/WallpaperPathRetrieval.cs:44-46
Timestamp: 2025-02-28T07:47:24.148Z
Learning: In Flow.Launcher's WallpaperPathRetrieval class, using a `using` statement with MemoryStream when loading images with BitmapImage does not work properly, even when using BitmapCacheOption.OnLoad. The stream needs to remain open while the bitmap is in use.
Learnt from: onesounds
PR: Flow-Launcher/Flow.Launcher#3394
File: Flow.Launcher/Themes/Darker Glass.xaml:134-141
Timestamp: 2025-03-28T21:20:54.978Z
Learning: In WPF applications like Flow.Launcher, Border elements cannot directly display text content and require a child element like TextBlock to handle text rendering. This separation of concerns (Border for visual container styling, TextBlock for text display) follows WPF best practices and provides greater styling flexibility.
Learnt from: onesounds
PR: Flow-Launcher/Flow.Launcher#3394
File: Flow.Launcher/Themes/Darker Glass.xaml:134-141
Timestamp: 2025-03-28T21:20:54.978Z
Learning: In WPF applications like Flow.Launcher, Border elements cannot directly display text content and require a child element like TextBlock to handle text rendering. This separation of concerns (Border for visual container styling, TextBlock for text display) follows WPF best practices and provides greater styling flexibility.
Learnt from: taooceros
PR: Flow-Launcher/Flow.Launcher#2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
🧬 Code Graph Analysis (3)
Flow.Launcher/MainWindow.xaml.cs (1)
Flow.Launcher.Plugin/EventHandler.cs (1)
ActualApplicationThemeChangedEventArgs
(91-97)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (1)
Flow.Launcher/PublicAPIInstance.cs (1)
IsApplicationDarkTheme
(591-594)
Flow.Launcher/ViewModel/MainViewModel.cs (1)
Flow.Launcher.Plugin/EventHandler.cs (1)
ActualApplicationThemeChangedEventArgs
(91-97)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Report (PR)
- GitHub Check: gitStream workflow automation
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (12)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (1)
599-603
: LGTM! Clean API addition.The method signature and documentation are well-defined and follow the existing interface patterns.
Flow.Launcher/PublicAPIInstance.cs (2)
591-594
: LGTM! Correct theme detection implementation.The implementation correctly uses
ThemeManager.Current.ActualApplicationTheme
to determine if the current theme is dark.
596-600
: LGTM! Proper event forwarding pattern.The event implementation correctly forwards to the MainViewModel, maintaining the established architectural pattern for event handling.
Flow.Launcher.Plugin/EventHandler.cs (1)
43-48
: LGTM! Consistent delegate pattern.The delegate follows the established naming and signature patterns used by other event handlers in this file.
Flow.Launcher/ViewModel/MainViewModel.cs (4)
25-25
: LGTM!The ModernWpf using statement is correctly added to support ThemeManager integration.
199-200
: LGTM!The event subscription is correctly placed in the constructor and follows proper initialization patterns.
837-837
: LGTM!The public event declaration is properly placed and correctly typed for exposing theme change notifications to consumers.
1992-1992
: LGTM!Proper event unsubscription in Dispose method prevents memory leaks and follows standard disposal patterns.
Flow.Launcher/MainWindow.xaml.cs (4)
23-23
: LGTM: Necessary using statement added.The
Flow.Launcher.Plugin
namespace is correctly added to support the newActualApplicationThemeChangedEventArgs
type used in the refactored event handler.
96-96
: LGTM: Good architectural improvement.Moving from static
ModernWpf.ThemeManager
event to the MainViewModel instance event improves encapsulation and follows MVVM pattern better. This change aligns well with exposing theme events through the public API.
105-108
: LGTM: Event handler properly updated for new event type.The method signature correctly matches the new
ActualApplicationThemeChangedEventArgs
event type. While theIsDark
property from args isn't used, this is likely intentional sinceRefreshFrameAsync()
can determine the current theme state internally.
1355-1355
: LGTM: Proper event cleanup in Dispose method.The event unsubscription correctly pairs with the constructor subscription, preventing potential memory leaks by ensuring the event handler is properly cleaned up during disposal.
Add Theme mode API Function
Add Theme mode API Function
Add API functions: